-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Translational arm joint support #996
Translational arm joint support #996
Conversation
…to convert protobuf floats to those inputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits and a question for future proofing - which is not a big deal and I can see an easy refactor if it's an issue down the line.
Q: this seems like a refactor. How does this support translational? Also, why put proto methods that are implemented once on an interface instead of keeping as a utility and ifc lean? |
@edaniels the main/only thing gating translational joint support prior to this PR, is that arm joint positions are passed in as a list of raw floats, which are currently assumed to be in degrees and are converted to radians. If something with a translational joint receives the joint positions proto message, it needs to know which of those raw floats should be converted from degrees to radians and which should not. The ultimate source of truth for what sort of value something expects is the thing ultimately consuming it, which is what is implemented here. The actual frames that represent a rotational movement will call utils.DegToRad when provided the protobuf value, while the translational ones will not. This also leaves the door open for someone to pass in some other sort of value and write their own transformation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's Get Translational Movement!
|
Note that this does not also include the API change, which is to simply rename "degrees" to "values" in JointPositions